docs: 4 concept pages (event sourcing, hexagonal, Result, multi-tenancy)#18
docs: 4 concept pages (event sourcing, hexagonal, Result, multi-tenancy)#18
Conversation
Flesh out the four /concepts/ pages so readers can ramp up on the framework's load-bearing ideas without diving into source first: event sourcing, hexagonal architecture, the Result pattern, and multi-tenancy. Each page includes a Mermaid diagram, 2-3 real code snippets pinned to fe1ab5b with GitHub permalinks, ADR cross-links, and links to neighbouring concept pages. Closes POM-183.
There was a problem hiding this comment.
Pull request overview
Replaces four placeholder “Concepts” documentation pages with substantive, code-referenced guides and updates the docs index to surface them prominently.
Changes:
- Adds a “Concepts” section to
docs/index.mdlinking to the four concept pages. - Rewrites the four concept pages with explanations, Mermaid diagrams, and permalinks to current code.
- Documents key framework patterns: event sourcing, hexagonal architecture, Result/Error, and multi-tenancy.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/index.md | Adds a “Concepts” section linking to the four new concept pages. |
| docs/concepts/event-sourcing.md | Replaces placeholder with an event-sourcing overview, rules, and referenced snippets/diagrams. |
| docs/concepts/hexagonal-architecture.md | Replaces placeholder with ports/adapters explanation, layer rules, and referenced snippets/diagrams. |
| docs/concepts/result-pattern.md | Replaces placeholder with Result/Error guidance, composition patterns, and anti-patterns. |
| docs/concepts/multi-tenancy.md | Replaces placeholder with tenant resolution/validation flow, context propagation, and isolation strategies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The case against exception-as-control-flow: | ||
|
|
||
| - **Invisibility in the type system.** A method that `throws` looks identical to one that doesn't. The compiler can't tell you that you forgot to handle a `StripeException` — only production can. Result types put failure on the signature where it can't be missed. | ||
| - **Cost on the hot path.** Throwing is expensive (stack capture, exception filters); on a request handling 10k events/minute it adds up. `Result` is a single allocation with no stack walk. |
There was a problem hiding this comment.
The performance claim here is factually off: Result/Result<T> are classes in Compendium.Core.Results (each call to Success/Failure allocates a new object), and failures also allocate an Error (which currently allocates a Dictionary for Metadata). Consider rephrasing to emphasize avoiding exception stack-unwinding/stacktrace costs rather than stating it's a “single allocation.”
| - **Cost on the hot path.** Throwing is expensive (stack capture, exception filters); on a request handling 10k events/minute it adds up. `Result` is a single allocation with no stack walk. | |
| - **Cost on the hot path.** Throwing is expensive (stack capture, exception filters, stack unwinding); on a request handling 10k events/minute it adds up. Returning `Result` avoids that exception machinery for expected outcomes. |
|
|
||
| ### 5. Don't use `string` errors | ||
|
|
||
| The implicit conversion from `string` to `Error` exists for ergonomics in trivial cases, but a string error has no `Code`, no `Type`, and no `Metadata`. Production errors should always be constructed via `Error.Validation(...)`, `Error.NotFound(...)`, etc., or via a per-context factory like `BillingErrors`. |
There was a problem hiding this comment.
This description of the string -> Error implicit conversion doesn’t match the current implementation: public static implicit operator Error(string error) creates Error.Failure("General.Failure", error), so the resulting error does have a Code and Type (and an empty Metadata dictionary). Suggest updating the wording to reflect that string errors collapse to a generic code/type and can’t carry structured metadata or specific error codes.
| The implicit conversion from `string` to `Error` exists for ergonomics in trivial cases, but a string error has no `Code`, no `Type`, and no `Metadata`. Production errors should always be constructed via `Error.Validation(...)`, `Error.NotFound(...)`, etc., or via a per-context factory like `BillingErrors`. | |
| The implicit conversion from `string` to `Error` exists for ergonomics in trivial cases, but it collapses the error to a generic failure (for example, code `General.Failure`, failure `Type`, and an empty `Metadata` dictionary). Production errors should always be constructed via `Error.Validation(...)`, `Error.NotFound(...)`, etc., or via a per-context factory like `BillingErrors`, so they can carry specific error codes and structured metadata. |
| Three policy knobs on `TenantConsistencyOptions`: | ||
|
|
||
| - `RequireAtLeastOneSource` — should anonymous (no-tenant) requests be allowed at all? Default `true`. | ||
| - `MinimumRequiredSources` — for high-sensitivity APIs you can demand 2 or 3 agreeing sources (e.g. JWT *and* subdomain) to harden against a single compromised credential. | ||
| - `ExcludedPaths` — health checks, metrics, and `.well-known` endpoints bypass tenant validation entirely. | ||
|
|
There was a problem hiding this comment.
ExcludedPaths is listed here as a knob on TenantConsistencyOptions, but the consistency validator doesn’t consult ExcludedPaths (path exclusion is handled by TenantValidationMiddlewareOptions.ExcludedPaths). Either adjust the text to point to the middleware options, or clarify how/where TenantConsistencyOptions.ExcludedPaths is applied (or remove it if it’s unused).
| Three policy knobs on `TenantConsistencyOptions`: | |
| - `RequireAtLeastOneSource` — should anonymous (no-tenant) requests be allowed at all? Default `true`. | |
| - `MinimumRequiredSources` — for high-sensitivity APIs you can demand 2 or 3 agreeing sources (e.g. JWT *and* subdomain) to harden against a single compromised credential. | |
| - `ExcludedPaths` — health checks, metrics, and `.well-known` endpoints bypass tenant validation entirely. | |
| Two policy knobs on `TenantConsistencyOptions`: | |
| - `RequireAtLeastOneSource` — should anonymous (no-tenant) requests be allowed at all? Default `true`. | |
| - `MinimumRequiredSources` — for high-sensitivity APIs you can demand 2 or 3 agreeing sources (e.g. JWT *and* subdomain) to harden against a single compromised credential. | |
| Path-based bypasses such as health checks, metrics, and `.well-known` endpoints are handled by `TenantValidationMiddleware` via its excluded-path configuration, before `ITenantConsistencyValidator` runs. |
|
Superseded by #21 (which I authored directly after the VK session stalled). Closing to avoid duplicate work. |
Summary
Replaces the four placeholder pages in
docs/concepts/with real, pedagogical content (~3-4 minutes each, ~15 minutes total). Each page covers the why, the moving parts, and the rules that keep the model honest, with real code snippets pinned to the currentmainSHA.event-sourcing.md(166 lines) — append-only log as source of truth,IDomainEvent,AggregateRoot<TId>, idempotent projections, snapshots. Mermaid diagram of the command → aggregate → events → store → projections flow. Real snippets fromIDomainEvent,AggregateRoot.AddDomainEvent,IProjection<TEvent>,OrderSummaryProjection.ApplyAsync.hexagonal-architecture.md(188 lines) — Core / Application / Adapters, ports vs adapters, the zero-dep Core rule, layer dependency table. Mermaid diagram of the hexagon with concrete adapter examples. Real snippets fromIBillingService(port) andStripeBillingService.CreateCheckoutSessionAsync(adapter) showing the catch-and-translate pattern.result-pattern.md(235 lines) —Result<T>,Error,ErrorType, functional combinators (Map/Bind/Match), the case against exceptions, five anti-patterns. Mermaid state diagram. Real snippets fromResult.cs(both base and generic),Error.cs, andResultExtensions.cs.multi-tenancy.md(226 lines) — three tenant sources (header, subdomain, JWT), consistency validation,AsyncLocal-backedTenantContext, isolation strategies table (DB-per-tenant / schema-per-tenant / RLS). Mermaid flow of the validation pipeline. Real snippets fromTenantConsistencyValidator,TenantValidationMiddleware, andTenantContext.docs/index.mdnow lists all four concept pages directly under a "Concepts" section. The existingdocs/concepts/toc.ymlalready pointed at the four files, so no TOC change was needed.All GitHub permalinks pin to
fe1ab5b7388a80f2d9b87bef9bcc543a6854be89(currentmainat PR open).Closes POM-183.
Test plan
Deploy Docsworkflow (docs-deploy.yml) builds without warnings on the four filesmainsite)../adr/0001-result-pattern.md→../adr/0005-event-sourcing-vs-state.md)